fix(broker): recover existing observer token on name-conflict mint - #1227
Conversation
POST /api/observer-token mints a fixed-name observer token per workspace
(Pear always uses "pear-dashboard-observer"), but relaycast enforces a
(workspace_id, name) unique index, so a second mint under the same name
fails with observer_token_name_conflict (409, relaycast#232 — not yet
released). Since callers have no way to know in advance whether a token
under that name already exists, repeat minting needs to just work.
When create_observer_token fails with that specific error code, fall back
to listing existing observer tokens for the workspace, finding the one
matching the attempted name, and rotating it to obtain fresh raw token
material — the only way to recover a usable value, since the original raw
token was never persisted anywhere. Any other create error (timeout,
network failure, a different API error) still propagates as a failure and
does not trigger this fallback. If no token matches the name despite the
conflict (e.g. a race with a concurrent revoke), the original conflict
error is propagated rather than papering over it.
Behavioral note: this means a caller that's minted before will get their
token rotated (invalidating the previous raw value) on every subsequent
mint through this path. Acceptable here since Pear's mintObserverToken
always treats a freshly-returned token as authoritative and re-caches it,
but flagging explicitly since it's a real, intentional side effect.
Adds RelaycastHttpClient::list_observer_tokens/rotate_observer_token
wrappers mirroring create_observer_token's existing pattern, and changes
all three to preserve the underlying RelayError (via anyhow::Error::from
instead of anyhow::anyhow!("{error}")) so the conflict code can be matched
structurally instead of via string matching. The list+rotate fallback
reuses the same http_api_observer_token_timeout() as the initial create
call, bounding it independently so a hung fallback can't block the runtime
task either.
Depends on relaycast#232 for the underlying 500->409 fix; until that ships
and relaycast-cloud picks it up, production mints will still 500 on
conflict rather than reaching this new fallback path.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR preserves structured Relaycast observer-token errors and adds a mint-or-recover flow that lists and rotates a compatible existing token after a 409 name conflict. The API route, timeout handling, tests, runtime exports, and changelog are updated. ChangesObserver token mint and recovery flow
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant API as ListenApiRequest
participant Mint as mint_or_recover_observer_token
participant Relaycast as RelaycastHttpClient
API->>Mint: Request observer token
Mint->>Relaycast: create_observer_token
Relaycast-->>Mint: Created token or 409 name conflict
Mint->>Relaycast: list_observer_tokens
Relaycast-->>Mint: Existing compatible token
Mint->>Relaycast: rotate_observer_token
Relaycast-->>API: Created or recovered token
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements a fallback mechanism for observer token minting when a name conflict occurs (HTTP 409 observer_token_name_conflict). Instead of failing, the broker now lists existing observer tokens, finds the conflicting token by name, and rotates it to recover a fresh, usable token. This change includes new methods on RelaycastHttpClient, helper functions and enums to manage the minting outcome, and comprehensive unit tests covering various success, error, and timeout scenarios. I have no further feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/broker/src/runtime/api.rs (1)
137-160: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winMisleading error message on recovery-path failure.
The failure arm reuses
"Failed to create observer token: {error}"even though the error here originates fromlist_observer_tokensorrotate_observer_token, notcreate_observer_token. This makes it harder to tell from logs/API responses whether the original create or the fallback list+rotate is what actually failed.💡 Suggested fix
match fallback { Ok(Ok(observer_token)) => Ok(ObserverTokenMintOutcome::RecoveredViaRotate(observer_token)), Ok(Err(error)) => Err(ObserverTokenMintError::Failed(format!( - "Failed to create observer token: {error}" + "Failed to recover existing observer token via list+rotate: {error}" ))), Err(_) => Err(ObserverTokenMintError::TimedOut), }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/broker/src/runtime/api.rs` around lines 137 - 160, The recovery failure in recover_observer_token_after_name_conflict is reporting the wrong operation, since the error comes from list_observer_tokens or rotate_observer_token rather than create_observer_token. Update the Err branch in recover_observer_token_after_name_conflict to emit a recovery-specific message that clearly names the fallback path (list+rotate) and preserves the underlying error, so logs and API responses distinguish original token creation failures from recovery-path failures.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@crates/broker/src/runtime/api.rs`:
- Around line 137-160: The recovery failure in
recover_observer_token_after_name_conflict is reporting the wrong operation,
since the error comes from list_observer_tokens or rotate_observer_token rather
than create_observer_token. Update the Err branch in
recover_observer_token_after_name_conflict to emit a recovery-specific message
that clearly names the fallback path (list+rotate) and preserves the underlying
error, so logs and API responses distinguish original token creation failures
from recovery-path failures.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 9401c965-a86a-4f2e-9d7a-d57c5172ee6d
📒 Files selected for processing (4)
crates/broker/src/relaycast/ws.rscrates/broker/src/runtime/api.rscrates/broker/src/runtime/mod.rscrates/broker/src/runtime/tests.rs
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 4 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 29ffaff267
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Resolve the tests.rs import-block conflict by unioning the observer-token mint/recover imports with main's dead-letter additions, and address PR review feedback: - Reconcile scopes/filters before rotating (cubic P1, codex P2): the list+rotate recovery now rotates an existing same-named token only when its scopes exactly match default_observer_token_scopes() and it carries no filters; otherwise the original observer_token_name_conflict propagates instead of handing back credentials with unexpected access. - Keep create+recover within the HTTP handler deadline (codex P2): the create call and the list+rotate fallback now share a single overall timeout budget via timeout_at, rather than the fallback getting a fresh full window that could overrun the 30s reply deadline. - Recovery-path failures now report "recover existing observer token via list+rotate" instead of the misleading "create observer token" (coderabbit nitpick). - Bump the timeout test budget 50ms -> 200ms so it can only trip inside the delayed list fallback, not the near-instant create (cubic P3). - Add a test covering scope-mismatch -> conflict propagation. Also drop a stray committed conflict marker from the [10.4.0] changelog section and add an [Unreleased] Fixed entry for the recovery behavior. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G9HTV1pYECN5tvoyRBTLUH
…Patch] main cut v10.5.0 and the merge carried this PR's changelog bullet under the released [10.5.0] heading. This change isn't in the 10.5.0 release, so move it to [Unreleased] and set the pending level to Patch (a bug fix). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G9HTV1pYECN5tvoyRBTLUH
2 participants
Summary
POST /api/observer-token(#1223) mints a scoped, read-only Relaycast observer token per workspace. Pear mints one per project using a fixed name (pear-dashboard-observer) every time it needs one — it has no way to know in advance whether a token under that name was already minted for that workspace.relaycast enforces a
(workspace_id, name)unique index on observer tokens. relaycast PR #232 (not yet released) fixes a bug where minting a second token under the same name crashed with an uncaught 500 instead of returning a clean409 observer_token_name_conflict. Once #232 ships, repeat mints will fail cleanly — but they'll still fail, which breaks Pear's re-mint-on-demand flow.This PR adds a fallback: when
create_observer_tokenfails specifically withobserver_token_name_conflict, list existing observer tokens for the workspace, find the one matching the attempted name, and rotate it to obtain fresh, usable raw token material (the original raw token was never persisted anywhere, so rotating is the only way to recover a usable value). The response shape is identical to a normal create.Any other error from
create_observer_token(timeout, network failure, a different API error) still propagates as a failure and does not trigger this fallback.A caller that's minted an observer token under a given name before will get that token rotated (invalidating whatever raw value was previously issued under that name) on every subsequent mint that hits this fallback path. This is intentional and unavoidable — the original raw value can't be recovered any other way. It's fine for this endpoint's known caller: Pear's
mintObserverToken(inbroker.ts) always treats a freshly-returned token as authoritative and re-caches it. Any other, currently-hypothetical holder of a previous raw value under the same name would silently lose access when this path triggers.Implementation
RelaycastHttpClient(crates/broker/src/relaycast/ws.rs) gainslist_observer_tokens()androtate_observer_token(id)wrappers, mirroring the existingcreate_observer_tokenwrapper's structure (SDK error handling viarelay_client()).relaycast::RelayErrorvia.map_err(anyhow::Error::from)(previouslyanyhow::anyhow!("{error}"), which discarded the source and made the error un-downcastable) so callers can match on the structured API error code instead of string-matchingDisplayoutput.crates/broker/src/runtime/api.rs(theListenApiRequest::CreateObserverTokenhandler) gainsmint_or_recover_observer_token, which wraps the create call, detectsobserver_token_name_conflictviaRelayError::code(), and on match callsrecover_observer_token_after_name_conflict(list + find-by-name + rotate). The matched token is rotated only when its scopes exactly equaldefault_observer_token_scopes()and it carries no filters — i.e. it matches what this endpoint itself mints; a same-named token with broader scopes or restrictive filters is treated as a non-match so the endpoint never hands back credentials with unexpected access or visibility. If no token matches the attempted name under that contract (e.g. a race with a concurrent revoke, or a scope/filter mismatch), the original conflict error is propagated rather than panicking or synthesizing a misleading response.http_api_observer_token_timeout()budget (one deadline viatimeout_at), rather than the fallback getting a fresh full window. This keeps total create+recover time bounded by that duration, so a successful recovery can't overrun the HTTP handler's 30s (LISTEN_API_SEND_TIMEOUT) reply deadline and get reported to the caller as a spurious timeout, and a hung fallback still can't block the runtime task indefinitely.Depends on
relaycast PR #232 (500→409 fix for the underlying name-conflict error) — not yet released. Until it ships and
relaycast-cloudpicks it up, production observer-token mints will still 500 on conflict rather than reaching this new fallback path. This PR's new behavior is not exercisable against production until then, but is fully covered by unit tests against a mocked relaycast API.Test plan
cargo check -p agent-relay-brokercargo fmt -p agent-relay-broker -- --checkcargo test -p agent-relay-broker --lib— 768 passed, 0 failed, 4 ignoredcrates/broker/src/runtime/tests.rs, all against a mocked relaycast HTTP API (httpmock):observer_token_name_conflict_falls_back_to_list_and_rotate— a 409observer_token_name_conflicton create triggers list+rotate and returns the rotated tokenobserver_token_non_conflict_error_does_not_trigger_fallback— a 403 on create propagates as a failure without ever calling list/rotateobserver_token_conflict_without_matching_name_propagates_original_error— a 409 whose name isn't found in the list propagates the original conflict error instead of panicking, and never calls rotate against an unrelated tokenobserver_token_conflict_with_mismatched_scopes_propagates_original_error— a 409 whose same-named token carries broader scopes than the endpoint mints propagates the original conflict error and never rotates itobserver_token_fallback_respects_the_supplied_timeout— a slow list response during the fallback still respects the shared timeout budget🤖 Generated with Claude Code
<img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg">